Skip to content

Fix duplicate OAuth code exchange causing invalid_grant crashes (MAILSPRING-CLIENT-37) - #2792

Open
bengotow wants to merge 2 commits into
masterfrom
claude/awesome-ritchie-14fdqm
Open

Fix duplicate OAuth code exchange causing invalid_grant crashes (MAILSPRING-CLIENT-37)#2792
bengotow wants to merge 2 commits into
masterfrom
claude/awesome-ritchie-14fdqm

Conversation

@bengotow

Copy link
Copy Markdown
Collaborator

Summary

Fixes MAILSPRING-CLIENT-37 — 111 events / 51 users, unresolved.

Sentry error:

Error: OAuth Code exchange returned 400 Bad Request: {"error":"invalid_grant","error_description":"AADSTS9002313: Invalid request. Request is malformed or invalid. ..."}

Stacktrace: oauth-signin-page.js:100 _onReceivedCode → onboarding-helpers.js:210 buildMicrosoftAccountFromAuthResponse → onboarding-helpers.js:44 fetchPostWithFormBody

Root cause

OAuthSignInPage (used for Gmail, Office 365, and Outlook sign-in) starts a local http.createServer in componentDidMount to receive the OAuth redirect. Its request handler called this._onReceivedCode(code) for every incoming request that carried a code query parameter, with no check for whether a code had already been received and processed:

this._server = http.createServer((request, response) => {
  if (!this._mounted) return;
  const code = extractOAuthCodeFromUrl(request.url);
  if (code) {
    this._onReceivedCode(code);   // <-- runs on every hit, not just the first
    response.writeHead(302, { Location: 'https://id.getmailspring.com/oauth/finished' });
    response.end();
  }
  ...
});

OAuth authorization codes are single-use. _onReceivedCode triggers a code exchange that can take several seconds (token exchange + Graph profile fetch + IMAP/SMTP connection test in finalizeAndValidateAccount), and during that window a second request to the same callback URL is plausible — a browser retry, a security product prefetching/scanning the redirect link, or a back/forward replay of history. When that happens, the second fetchPostWithFormBody call resends the already-consumed code, and Microsoft's identity platform rejects it with invalid_grant / AADSTS9002313 ("Invalid request. Request is malformed or invalid"), which is exactly the error reaching Sentry. The same code path is shared by Gmail and Outlook, so the same failure mode applies there too (surfacing as the provider-specific equivalent of a reused-code error).

I verified there's no other issue at play here: the redirect_uri sent during the initial auth request and the token exchange match exactly, and the PKCE code_verifier/code_challenge pair is generated once and stays consistent between the two requests. The systemic, repeated nature of the error (111 occurrences across 51 distinct users) also fits a structural double-invocation bug better than users simply being slow to complete the flow.

Fix

Add a _codeReceived guard so only the first request carrying a code triggers _onReceivedCode(). Later requests (e.g. a duplicate hit) still get the same 302 redirect response, so the browser experience is unaffected — we just stop re-submitting an already-consumed code to the provider.

Test plan

  • Read through the full onboarding OAuth flow (oauth-signin-page.tsx, onboarding-helpers.ts) to confirm redirect_uri and PKCE code_verifier/code_challenge are consistent between the auth request and token exchange, ruling those out as the cause.
  • Confirmed OAuthSignInPage is shared by Gmail, Office 365, and Outlook sign-in (page-account-settings-gmail.tsx, page-account-settings-o365.tsx, page-account-settings-outlook.tsx), so the fix covers all three.
  • Manual verification of the full sign-in flow requires live OAuth credentials for a provider and isn't practical in this environment; the change is a minimal, isolated guard around existing logic.

🤖 Generated with Claude Code


Generated by Claude Code

…LSPRING-CLIENT-37)

The local callback server in OAuthSignInPage re-invoked _onReceivedCode()
for every request that carried a `code` query param, with no check that
the code had already been submitted. Browsers/security software can hit
the callback URL more than once for the same redirect (retries, link
prefetching, back/forward replay), which resends an authorization code
that's already been exchanged. Since auth codes are single-use, the
second exchange attempt fails with invalid_grant (AADSTS9002313 on
Microsoft, and the equivalent on other providers), which is what was
reaching Sentry.

Add a _codeReceived guard so only the first code received triggers
_onReceivedCode(); the server still returns the redirect response for
any later hits so the browser doesn't show an error.
@indent-staging

indent-staging Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Warning

Indent Zero is shutting down on August 7th. Please migrate over to Indent 2.0 to continue getting PR reviews.

PR Summary

Prevents the local OAuth callback server in OAuthSignInPage from re-invoking _onReceivedCode() for duplicate hits to the redirect URL (browser retries, prefetching, security software), which was producing single-use-code invalid_grant errors reaching Sentry. The follow-up commit refines the guard so it only dedupes identical codes, keeping the legitimate "go back and sign in with a different account" flow working.

  • Added _lastCodeReceived: string | null field tracking the last accepted code
  • Wrapped _onReceivedCode(code) in if (isNewCode && !isBusy), where isBusy is authStage === 'buildingAccount' || 'accountSuccess'
  • Keeps the 302 → id.getmailspring.com/oauth/finished response outside the guard so every duplicate callback still sees the finished page
  • Retry path still works: on error stage the same code is ignored but a genuinely new code from a fresh sign-in is accepted

Issues

No issues found.

CI Checks

The test job failed during npm ci because downloading the Electron binary returned HTTP 503 from github.com/electron/electron/releases/.... This is a transient GitHub release CDN outage, unrelated to the PR changes — re-running the workflow should resolve it.

Failing test
  • npm ci failed while running the electron postinstall (node install.js) because the Electron zip download from GitHub Releases returned 503 Service Unavailable. This is a transient network/CDN error, not something introduced by this PR. Re-run the failed job to retry.

Custom Rules 3 rules evaluated, 3 passed, 0 failed

Passing This is a longer title to see what happens when they are too long to fit
Passing B
Passing Ben Rule

View all rules

@indent

indent Bot commented Aug 12, 2026

Copy link
Copy Markdown

Warning

This organization's free trial has ended, so Indent couldn't start this review. Manage billing to resume reviews on this repository.

@bengotow

Copy link
Copy Markdown
Collaborator Author

@claude can you make sure that this resets if the user goes back, then forward again in the auth flow, triggering a different code callback? Maybe the state variable could store the last code received so we can accept a new/different code, if this is a possible scenario.

Per review feedback: the previous guard used a one-shot boolean, which
would incorrectly block a legitimate second code if the user goes back
in their browser and completes sign-in again with a different account
after an error (the server keeps listening as long as the component is
mounted, which includes the 'error' authStage while waiting for the
user to click "Try Again"). Track the last code received instead, so an
identical repeat is still ignored (the original invalid_grant fix) but
a genuinely different code is accepted — unless we're already mid-
exchange or have already succeeded for a prior code.

Copy link
Copy Markdown
Collaborator Author

Good catch — that scenario is real. The server keeps listening for as long as the component is mounted, which includes the error authStage (waiting for the user to click "Try Again"), so a user who goes back in their browser and completes sign-in again with a different account after an error would have hit the one-shot guard.

Pushed a fix in bf998bc: swapped the boolean for _lastCodeReceived: string | null, so we only ignore a code if it's identical to the one we last started exchanging (the original invalid_grant case), and also skip starting a new exchange while one is already mid-flight or has just succeeded (buildingAccount / accountSuccess) to avoid two concurrent exchanges racing. A genuinely different code — e.g. from going back and picking another account — is now accepted.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

The test check failure on bf998bc isn't related to this change — npm ci failed during electron's postinstall because GitHub's release CDN returned a transient 503 Service Unavailable while downloading electron-v41.7.2-linux-x64.zip:

npm error HTTPError: Response code 503 (Service Unavailable) for https://github.com/electron/electron/releases/download/v41.7.2/electron-v41.7.2-linux-x64.zip

That's an infrastructure hiccup unrelated to the diff. I don't have permission to re-run the workflow from here (got a 403), so it'll need a maintainer to re-run the job, or a new push will retrigger it.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants